Repository navigation
fix: Make TestData value_for_all return its value when targeting is off - #567
Merged
Merged
Conversation
Generated-by: termag
keelerm84
marked this pull request as ready for review
November 5, 2026 19:14
joker23
approved these changes
Nov 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FlagBuilder.value_for_all(inTestData) andFlagBuilderV2.value_for_all(inTestDataV2) replaced the flag's variations with the single value and calledvariation_for_all(0), which leaves the off variation unchanged. A new test flag is a boolean flag with off variation 1, so the built flag hadvariations: [value]andoffVariation: 1. That index is out of range, so the flag failed to evaluate (malformed flag) when targeting was off. The method is documented to return the value whether targeting is on or off.value_for_allnow also sets the off variation to 0 in both builders.variation_for_allis unchanged; it still leaves the off variation alone.The new tests check the built flag with targeting on and off for both builders, and check that the client evaluates a
value_for_allflag with targeting off to the value with anOFFreason.This matches the Ruby fix in launchdarkly/ruby-server-sdk#489.
Opened from termag
Note
Overview
Fixes TestData / TestDataV2 so
value_for_allbehaves as documented when targeting is off.Previously,
value_for_allreplaced variations with a single value and set fallthrough viavariation_for_all(0), but left off variation at the default boolean flag index (1). With only one variation, that index was invalid and evaluation failed when targeting was off. The builders now also calloff_variation(0)so the off path returns the same value.Tests cover the built flag shape (including targeting off) for both builders, plus a client evaluation test asserting value
'x'with reasonOFFwhen targeting is disabled.Reviewed by Cursor Bugbot for commit d9958e4. Bugbot is set up for automated code reviews on this repo. Configure here.